compiler-builtins is developed in its own repository. If possible, consider making this change to rust-lang/compiler-builtins instead.

cc @tgross35

@rfcbot merge

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot reviewed

I agree that the lint on positional arguments isn't a blocker, though I think it's highly desirable.

operand before template

I don't think this is a problem, since it's not likely that people will expect it to work.

@folkertdev, does this just allow it for cfg or does it allow it for all attributes? E.g., would an attribute macro that expanded to cfg work here? If not, are there any other places currently in the language with a similar restriction?

Macros are not currently expanded, e.g.

#![feature(asm_cfg)]

macro_rules! annotated_line {
    () => {
        #[cfg(true)]
        "ret"
    };
}

#[unsafe(no_mangle)]
pub fn square() {
    unsafe {
        std::arch::asm!(
            #[cfg(true)]
            "",
            annotated_line!(),
            #[macros::expand_to_cfg(true)]
            "",

        )
    }
}

fn main() {}

emits

error: this attribute is not supported on assembly
  --> src/main.rs:17:13
   |
17 |             #[macros::expand_to_cfg(true)]
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0658]: attributes on expressions are experimental
  --> src/main.rs:5:9
   |
 5 |         #[cfg(true)]
   |         ^^^^^^^^^^^^
...
16 |             annotated_line!(),
   |             ----------------- in this macro invocation
   |

I don't think we currently allow attributes on arguments to builtin macros. E.g. cfg is not allowed on the template parameter of println! etc.

I'll look at what we can do here, but I suspect it'll be tricky.

I don't think we currently allow attributes on arguments to builtin macros. E.g. cfg is not allowed on the template parameter of println! etc.

It seems we do allow attributes here.

println!(#[allow(unused)] ""); //~ OK
println!(#[cfg(true)] ""); //~ OK

We don't allow removing expressions here though.

println!(#[cfg(false)] ""); //~ ERROR

(Edit: When I had tested this, I was in a file where I had enabled stmt_expr_attributes for an unrelated reason and didn't notice.)

reference PR: forthcoming (though maybe none is needed?)

The relevant parts of the Reference are attributes.allowed-positions, cfg.attr.allowed-positions, and cfg.cfg_attr.allowed-positions.

The latter two say:

The cfg attribute may be used anywhere attributes are allowed.

The cfg_attr attribute may be used anywhere attributes are allowed.

What would be a bit less than elegant would be to have to say, "the cfg/cfg_attr attributes may be used anywhere attributes are allowed and on arguments to the following macros...". It'd be more elegant if we could add arguments to these macros to the list of places that attributes are allowed and then keep the allowed-positions rules on cfg/cfg_attr untouched.

@folkertdev, does this just allow it for cfg or does it allow it for all attributes? E.g., would an attribute macro that expanded to cfg work here? If not, are there any other places currently in the language with a similar restriction?

cfg in asm are a part of asm's DSL mimicking attributes, and are entirely unrelated to cfg attributes from the language proper.
Not sure whether it's a good style or not.
Technically they could use some other syntax, and still work in the same way as now.

So anything that these "attributes" can potentially do (eager expansion or something else) needs to be implemented manually by the asm macro and is not guaranteed to match behavior of real attributes in language-supported expansion points.

It seems we do allow attributes here.

println!(#[allow(unused)] ""); //~ OK
println!(#[cfg(true)] ""); //~ OK

This is really an implementation detail. Actually println!(#[cfg(true)] "") does emit

error[E0658]: attributes on expressions are experimental
 --> src/main.rs:3:9
  |
3 |         #[cfg(true)]
  |         ^^^^^^^^^^^^
  |
  = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information

The allow attribute is just already allowed on expressions (so that error message is not complete).

We do actually parse arbitrary attributes on the assembly arguments, but anything besides cfg and cfg_attr is rejected with:

error: this attribute is not supported on assembly
 --> src/main.rs:3:9
  |
3 |         #[bar]
  |         ^^^^^^

This is really an implementation detail. Actually println!(#[cfg(true)] "") does emit

Oops... when I tested this, I was in a file where I'd enabled stmt_expr_attributes for an unrelated reason (testing closure capture behavior with rustc_capture_analysis) and didn't notice.

So just to summarize: using procedural macros on assembly is rejected when the asm! macro is expanded. Arbitrary attributes parse on lines of assembly, but only cfg and cfg_attr are allowed by the expansion. We can expand the set of accepted attributes over time (e.g. rustfmt::skip or allow(...) also seem OK.

Accepting procedural macros that expand to cfg (so, eager expansion), seems difficult and not that necessary. We're not blocking adding it in the future though.

A macro-by-example macro that expands to a string literal with a cfg currently is also rejected. When attributes on expressions are stabilized, that will need some attention (not just for asm!, but also for the format_args! family of macros, possibly for others.

So I don't see blockers here for this particular feature.

Thanks. Agreed. For my part, I want to see the Reference PR, then I'll check a box.

On the Reference PR, we need to update the grammar in:

• https://doc.rust-lang.org/nightly/reference/inline-assembly.html#r-asm.syntax

We need to add this to the allowed position list in:

• https://doc.rust-lang.org/nightly/reference/attributes.html#r-attributes.allowed-position

(The item for this should link to the rule described below.)

Let's please write out the rules you just mentioned, about what's accepted and what is not, in:

• https://doc.rust-lang.org/nightly/reference/inline-assembly.html#r-asm.ts-args

It'll be a new rule, e.g. asm.ts-args.attributes.

(Edit: Actually, it'll need to go elsewhere... we'll work that out in the Reference PR.)

If you have any general questions about this, please reach out in Zulip in t-lang-docs/reference.

The reference PR is up at rust-lang/reference#2063